-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Port the proc macro attributes to the new attribute parsing infrastructure #143607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Port the proc macro attributes to the new attribute parsing infrastructure #143607
Conversation
|
||
// Not a built-in macro | ||
None => (None, helper_attrs), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here should be equivalent to the logic above, other than that this is a bit more readable and that we use the already parsed attribute rather than parsing it from scratch
@@ -57,7 +57,7 @@ | |||
// see gated-link-args.rs | |||
// see issue-43106-gating-of-macro_escape.rs for crate-level; but non crate-level is below at "2700" | |||
// (cannot easily test gating of crate-level #[no_std]; but non crate-level is below at "2600") | |||
#![proc_macro_derive()] //~ WARN `#[proc_macro_derive]` only has an effect | |||
#![proc_macro_derive(Test)] //~ WARN `#[proc_macro_derive]` only has an effect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that a breaking change happened here! The code before this change would now produce an error.
Previously a proc_macro_derive
applied to the crate could have any arguments it wants, it was not checked. This PR fixes this bug, and this now errors.
I discussed this privately with @jdonszelmann, and she said this is fine to change, though we can consider doing a crater run for this just to make sure if you wish.
Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in compiler/rustc_attr_data_structures |
This comment has been minimized.
This comment has been minimized.
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This could use a little squash I think |
Not everything, but some commits are a bit small |
a6b7f55
to
432cde9
Compare
@jdonszelmann I've squashed things down a bit. I'm experimenting a bit with what commit size is best for reviewability, this was clearly too much of a good thing :P |
432cde9
to
7a60c84
Compare
r? @jdonszelmann |
|
☔ The latest upstream changes (presumably #143645) made this pull request unmergeable. Please resolve the merge conflicts. |
2b42dae
to
f8c757f
Compare
^ Rebased |
Port the proc macro attributes to the new attribute parsing infrastructure Ports `#[proc_macro]`, `#[proc_macro_attribute]`, `#[proc_macro_derive]` and `#[rustc_builtin_macro]` to the new attribute parsing infrastructure for #131229 (comment) I've split this PR into commits for reviewability, and left some comments to clarify things I did 4 related attributes in one PR because they share a lot of their code and logic, and doing them separately is kind of annoying as I need to leave both the old and new parsing in place then. r? `@oli-obk` cc `@jdonszelmann`
@traviscross what's the proper way to notify T-lang on this? What label, and could you add that? |
@rfcbot reviewed |
1 similar comment
@rfcbot reviewed |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
☔ The latest upstream changes (presumably #144109) made this pull request unmergeable. Please resolve the merge conflicts. |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
Signed-off-by: Jonathan Brouwer <[email protected]>
8166ced
to
97b6521
Compare
Rebased. As stated above I think @jdonszelmann already reviewed this so I think this is ready to be merged? |
FCP is complete, verified this doesn't need changes in the Reference, and reviewed the differences in the rebase, which look reasonable, so... @bors r=jdonszelmann,traviscross rollup |
Ports
#[proc_macro]
,#[proc_macro_attribute]
,#[proc_macro_derive]
and#[rustc_builtin_macro]
to the new attribute parsing infrastructure for #131229 (comment)I've split this PR into commits for reviewability, and left some comments to clarify things
I did 4 related attributes in one PR because they share a lot of their code and logic, and doing them separately is kind of annoying as I need to leave both the old and new parsing in place then.
r? @oli-obk
cc @jdonszelmann